Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jochem-brouwer
left a comment
There was a problem hiding this comment.
Cool! Left three comments with some pointers :)
| 0xa4: dynamicGasOp('LOG'), | ||
|
|
||
| // '0xf0' range - extended range/width modular arithmetic | ||
| 0xc0: asyncAndDynamicGasOp('SETMODX'), |
There was a problem hiding this comment.
This will add these opcodes as available from all EVMs at all hardforks (so these opcodes are now available at Frontier / Chainstart)
There was a problem hiding this comment.
I've removed the declarations here and only left the ones in eipOpcodes, I think that should fix this so that they only activate via eip activation via common.
| // arithmetic | ||
| 0xc3: asyncAndDynamicGasOp('ADDMODX'), | ||
| 0xc4: asyncAndDynamicGasOp('SUBMODX'), | ||
| 0xc5: asyncAndDynamicGasOp('MULMODX'), |
There was a problem hiding this comment.
Note: for dynamic gas ops, a gas method should be provided in gas.ts, otherwise the Interpreter will crash (it expects an entry there if the opcode is marked as dynamic)
| eips: [4762, 6800], | ||
| }, | ||
| evmmax: { | ||
| eips: [6690], |
There was a problem hiding this comment.
I don't think we should do an evmmax hardfork for this: if EVMMAX should be activated then the 6690 EIP should be activated (so instantiate a common and activate 6990 to use EVMMAX). I might have missed something from the EIP though. It seems to me that the EIP introduces new opcodes?
There was a problem hiding this comment.
Yes, currently 6 new opcodes are added upon evmmax activation.
This change is a work in progress implementation of the evmmax opcodes as defined in the latest EIP 6690 spec along with the geth reference implementation.
TODO:
FieldContextFieldContextwith evm to implement opcodesFieldContext